Repository navigation
Print null checks with the precedence of === - #9291
Conversation
Changelog🐛 Bug fix
Semantic Diff1 commit · Full page ✏️ Print null checks with the precedence of ===b2d750b · mununki · Nov 10, 2026, 14:29 UTC · edited
JS modified (operand of a null check) let pick = (c, a: Nullable.t<int>, b: Nullable.t<int>) => Nullable.isNullable(c ? a : b)
let sum = (a: int, b: int) => Nullable.isNullable(Nullable.make(a + b))
function pick(c, a, b) {
- return c ? a : b == null;
+ return (
+ c ? a : b
+ ) == null;
}
function sum(a, b) {
- return a + b | 0 == null;
+ return (a + b | 0) == null;
}JS modified (null check as an operand) let equal = (a, b, eq) => Option.equal(Nullable.toOption(a), Nullable.toOption(b), eq)
function equal(a, b, eq) {
- return Stdlib_Option.equal((a == null) ? undefined : Primitive_option.some(a), (b == null) ? undefined : Primitive_option.some(b), eq);
+ return Stdlib_Option.equal(a == null ? undefined : Primitive_option.some(a), b == null ? undefined : Primitive_option.some(b), eq);
}Review
|
✅ Compiler benchmarksAllocated wordsIn total: 682,345,601 → 682,345,575 (±0.00%). Generated projects: 923,215,341 → 923,214,496 (±0.00%). None of the 265 benchmarks changed by 0.1% or more. InstructionsIn total: 9,666,073,620 → 9,666,124,075 (±0.00%). None of the 33 benchmarks changed by 0.1% or more. Output sizeIn total: 25,812,251 → 25,812,323 (±0.00%). None of the 13 benchmarks changed by 0.1% or more. Pull request b2d750b compared with base 32116b5 in this run. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9291 +/- ##
=======================================
Coverage 81.95% 81.96%
=======================================
Files 456 456
Lines 65612 65613 +1
=======================================
+ Hits 53775 53781 +6
+ Misses 11837 11832 -5
🚀 New features to boost your workflow:
|
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
b00a4b5 to
2f2e9cf
Compare
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-9291 |
Why:
`Nullable.isNullable(e)`, and any other `%is_nullable`, compiles to the
JS IR node `Is_null_or_undefined e`, which `Js_dump` prints as
`e == null`. It printed `e` at level 1, below the precedence of `==`,
so an operand that binds more loosely lost its parentheses and the
check applied to part of it: `Nullable.isNullable(c ? a : b)` compiled
to `c ? a : b == null`, which JavaScript reads as `c ? a : (b == null)`.
The same happened to `||`, `&&` and bitwise operands, and an object in
an arrow function's body gave a syntax error. It also put the whole
check in parentheses wherever it was not at the top level, which the
output of `Stdlib_Nullable` shows. Both now follow
`Js_op_util.op_prec EqEqEq`, as `x === null` does: parentheses when the
level is above that of `==`, and the operand at its left level.
JS modified (operand of a null check):
Example:
let pick = (c, a: Nullable.t<int>, b: Nullable.t<int>) => Nullable.isNullable(c ? a : b)
let sum = (a: int, b: int) => Nullable.isNullable(Nullable.make(a + b))
Before (js):
function pick(c, a, b) {
return c ? a : b == null;
}
function sum(a, b) {
return a + b | 0 == null;
}
After (js):
function pick(c, a, b) {
return (
c ? a : b
) == null;
}
function sum(a, b) {
return (a + b | 0) == null;
}
JS modified (null check as an operand):
Example:
let equal = (a, b, eq) => Option.equal(Nullable.toOption(a), Nullable.toOption(b), eq)
Before (js):
function equal(a, b, eq) {
return Stdlib_Option.equal((a == null) ? undefined : Primitive_option.some(a), (b == null) ? undefined : Primitive_option.some(b), eq);
}
After (js):
function equal(a, b, eq) {
return Stdlib_Option.equal(a == null ? undefined : Primitive_option.some(a), b == null ? undefined : Primitive_option.some(b), eq);
}
Review: Is the precedence of `===` right for `==`? Both are equality
operators with the same precedence and left associativity, so a
context that takes `a === b` without parentheses takes `a == null`
too: arguments, `let` initializers, `?:`, `||`, `&&`, bitwise operators
and the left side of `===`. The right side of `===` and the operand of
`!` still get parentheses, as in `y === (x == null)` and
`!(x == null)`.
Review: Which checked-in outputs change? Only `Stdlib_Nullable`,
`Stdlib_TempTests` and `optional_external_arg_once_test`, where the
test of `?:` loses its parentheses, and `Primitive_option`, where
`x == null` as an operand of `||` does. No checked-in output had an
operand that lost them.
Review: Why does the conditional break over lines? `Cond` prints its own
parentheses that way when it needs them, as elsewhere in the output.
Review: What else prints an operand below its precedence? `In` prints both
operands at level 0 (#9222), `Typeof` puts itself in no parentheses,
and `Fun` ignores the level. They are separate fixes.
Signed-off-by: mununki <woonki.moon@gmail.com>
2f2e9cf to
b2d750b
Compare
Fixes: #9290
Nullable.isNullable(e), and any other%is_nullable, compiles toe == null.Js_dumpprintedebelow the precedence of==, so an operand that binds more loosely lost its parentheses and the check applied to part of it:Nullable.isNullable(c ? a : b)compiled toc ? a : b == null, andNullable.isNullable(Nullable.make(a + b))toa + b | 0 == null. The same happened with||,&&and bitwise operands, and with a conditional from aniforswitchexpression, an inlined function or aletused once. An object in an arrow function's body gave a syntax error. The check itself was put in parentheses wherever it was not at the top level, as in(a == null) ? undefined : …, which came up in the review of #9288.Is_null_or_undefinedis now printed withJs_op_util.op_prec EqEqEq, asx === nullis: in parentheses when the level is above that of==, and with its operand at the left level of===.!(x == null)and the right side of===,y === (x == null), keep their parentheses.Stdlib_Nullable(.mjsand.cjs),Stdlib_TempTests.mjsandoptional_external_arg_once_test.mjs(from Evaluate optional @obj arguments in source order and read properties once #9283), where the test of?:loses its parentheses, andPrimitive_option(.mjsand.cjs), where(x == null) ||from Pass undefined for an optional external argument or JSX key that is None #9288 becomesx == null ||. No checked-in output had an operand that lost them.In,TypeofandFunalso print operands below their precedence. They are left for separate fixes;Inis part ofDict.hasevaluates the key before the dict, and a comma expression operand changes what it tests #9222.Tests:
is_nullable_paren_test.reschecksNullable.isNullableof a conditional, anifexpression,||, an int sum and an object in an arrow body, and the check as an operand of||,?:and===. Compiled with master, the module fails to load with aSyntaxError, and without the object case four of its checks return wrong values.Checklist
changes/README.md), or no release notes are needed and a maintainer should applychangelog:skip.CONTRIBUTING.md).git commit -s).